Skip to content

feat!: bump engines to Node.js >=22.12.0 - #312

Merged
erickzhao merged 20 commits into
mainfrom
node-22
Feb 18, 2025
Merged

feat!: bump engines to Node.js >=22.12.0#312
erickzhao merged 20 commits into
mainfrom
node-22

Conversation

@erickzhao

@erickzhao erickzhao commented Jan 11, 2025

Copy link
Copy Markdown
Member

BREAKING CHANGE: bumps minimum Node.js version to >=22.12.0

ESM

Node.js 22.12 gives us the ability to use CommonJS require calls on ESM dependency graphs. This compatibility gives us the ability to migrate our libraries to ESM and maintain support for projects that are using CommonJS.

Removal of deprecated APIs

@electron/get already had a few deprecated APIs and this PR removes them since we're already making a breaking change anyways:

  • nightly_mirror option was removed (was deprecated in favour of nightlyMirror)
  • force option was removed (was deprecated in favour of cacheMode=WriteOnly).

Test migration from Jest to Vitest

Jest currently has issues with require(esm) while it works out of the box with Vitest. We were already looking to migrate to Vitest across other Ecosystem repos, so this PR just switches over.

  • Need to import functions from vitest (no globals)
  • jest.mock calls turned into vi calls
  • Regenerated snapshots

tsconfig.json changes

We're now basing our TypeScript config off of @tsconfig/node22, which is a bit different from what we had in the past.

Due to going ESM-only, we only have a singular tsconfig.json now.

Comment thread src/Cache.ts
d('* Replacing existing file');
await fs.remove(cachePath);

if (!fs.existsSync(path.dirname(cachePath))) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this new clause is needed because fs.rename will fail if the target directory doesn't exist, while fs.move creates the directory.

This also doesn't work across disks, but we could catch the resulting error and fall back to an fs.copy call if necessary.

@erickzhao
erickzhao marked this pull request as ready for review January 13, 2025 21:49
@erickzhao
erickzhao requested a review from a team as a code owner January 13, 2025 21:49
Comment thread package.json
@erickzhao
erickzhao requested a review from dsanders11 January 20, 2025 18:26
@erickzhao erickzhao changed the title feat!: bump engines to Node.js 22 feat!: bump engines to Node.js >=22.12.0 Jan 22, 2025
artifactName: 'electron',
mirrorOptions: {
mirror: 'https://mirror.example.com/',
// eslint-disable-next-line @typescript-eslint/camelcase

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank god

Comment thread tsconfig.json

@dsanders11 dsanders11 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, other than the declaration: true change in tsconfig.json that @erikian called out.

@erickzhao

erickzhao commented Feb 11, 2025

Copy link
Copy Markdown
Member Author

0f540cd adds declarations back and uses util.promisify because graceful-fs unfortunately doesn't patch fs.promises as per isaacs/node-graceful-fs#160

@dsanders11

Copy link
Copy Markdown
Member

and uses util.promisify because graceful-fs unfortunately doesn't patch fs.promises

Sad, but good catch!

@VerteDinde VerteDinde left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excited to get this landed!

@erickzhao
erickzhao merged commit 8be1f8d into main Feb 18, 2025
@erickzhao
erickzhao deleted the node-22 branch February 18, 2025 22:49
@continuous-auth

Copy link
Copy Markdown

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants